add dummy tone function in case no network connection for waston#49
add dummy tone function in case no network connection for waston#49irisdingbj wants to merge 1 commit intoIBM:masterfrom irisdingbj:dummytone
Conversation
irisdingbj
changed the title
| return "" | ||
| } | ||
|
|
||
| //getDummyTone is a dummy version for the tone anylyzer |
There was a problem hiding this comment.
Can you add some comments here to explain the thinking behind the HasPrefix checks? Meaning, why do you assume a word that starts with "s" is happy? And how does this work when you look for 's' on 275 and 281?
There was a problem hiding this comment.
hmmm.. I just want to return something to simulate tone analyzer which can show we call our version2 guest book.
Line 281 is a typo, I should use other beginning instead of "s"， eg: "w"-->worry
There was a problem hiding this comment.
could we do it based on certain words? Seems hard to be based on a prefix...
Also, I'd prefer return some indication that we are using dummy tone to generate the tone so we know it is not watson that is so dumb. :)
|
I think we do something similar for our redis stuff... meaning, if we don't see the REDIS env vars then we assume we're using the in-memory one - so in principle I'm ok with this. However, rather than faulting on the POST which could be due to the Watson service not behaving correctly - or waiting for the POST to time-out, should we do something else? I was thinking that we should change the "findRedisURL" func into something more generic (like @irisdingbj in these situations, what will be missing? Will the analyzer service/image be deployed? Based on this info we can decide what the check should be.... look for service env var (like we do with redis) or check for a network issue connecting to the service.... |
|
@duglin sounds good to put the check for waston tone analyzer before hand and use the flag to switch between the waston one and dummmy one. After that, what do you think we should use for dummy one? As I replied in your review comments, the current version is just for 'play' |
| if err != nil { | ||
| return "Error talking to tone analyzer service: " + err.Error() | ||
| return getDummyTone(value) | ||
| //return "Error talking to tone analyzer service: " + err.Error() |
There was a problem hiding this comment.
Let's make sure user sees from guestbook UI the value is returned from dummy tone service as unable to talk to watson.
|
@irisdingbj have you addressed all comments? is this ready to be merged? |
3 participants
…g workshop